Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config option to provide names for lstinline-like commands #126

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jprotze
Copy link
Contributor

@jprotze jprotze commented Nov 10, 2017

This PR adds the possibility to configure customized lstinline-like commands/shortcuts.
For something like:
\newcommand\myJSsnippet{\lstinline[style=JavaScript,breaklines=true]}
You would add this to the config:
LSTINLINEENV=myJSsnippet

latexdiff Outdated
@@ -2855,7 +2862,8 @@ sub postprocess {
###if ( defined($packages{"listings"} and $latexdiffpreamble =~ /\\RequirePackage(?:\[$brat0\])?\{color\}/)) {
# mark added verbatim commands
$addblock =~ s/\\DIFverb/\\DIFDIFaddverb/g;
$addblock =~ s/\\DIFlstinline/\\DIFDIFaddlstinline/g;
1 while $addblock =~ s/((?<!\\)%.*)\\DIF($LSTINLINEENV)/$1\\DIFDIFadd$2/mg;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably overlook something simple but why did you add the line 1 while $addblock ... .
As far as I can tell without testing this will change \DIFlstinline (and variants) in comments into \DIFDIFaddlstinline.
But the following line makes this change everywhere (including comments).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I missed to remove this line. Commented inline-code is handled at 3 other places now.

Copy link
Owner

@ftilmann ftilmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, if multiline \lstinline is indeed not allowed (I am not so familar with this package). There is one line I have a question about.
Also, is it really necessary to have a special treatment for lstinline commands in comments?
I would have thought that no harm is done if in comments the hashing is done and then later inserted again.

@jprotze
Copy link
Contributor Author

jprotze commented Nov 10, 2017

If the comment changed, I have seen that in some cases the color-highlighting was added. With the first part of this replacement being in comment, the result would not compile.

I'm checking, whether L3088 is sufficient to catch all cases.

@jprotze
Copy link
Contributor Author

jprotze commented Nov 10, 2017

For the restriction of lstinline to single line, I could not find something in the documentation. But my latex compiler complains with Package Listings Error: lstinline ended by EOL If I have a linebreak in the inline code. According to a test, \verb has the same limitation.

@ftilmann
Copy link
Owner

ftilmann commented Dec 4, 2017

The full pull request patched latexdiff failed with testsuite/verbatim-{old,new}.tex so I cannot accept as is.
I will look into this and isolate the actual commit causing this problem, as at least the adding of the config option should work smoothly and adds useful functionality but it will take a bit of time.

@jprotze
Copy link
Contributor Author

jprotze commented Feb 26, 2018

We found that lstinline has poor performance and triples compile time for our pdf compared to verb or simply a command containing \detokenize. The latter is also compatible with \DIFdel.

Nevertheless, I could find the bug and can add the changes if this is still of interest.

@ftilmann ftilmann added the Low label Jul 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants